Compare optional property flag when comparing against discriminant properties under exactOptionalPropertyTypes#61682
Conversation
…operties under `exactOptionalPropertyTypes`
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue (#61678) in which optional property flags are compared incorrectly against discriminant properties when the compiler flag exactOptionalPropertyTypes is enabled. Key changes include updated test cases that expect errors in union discriminant assignments and a modification of the comparison logic in the checker.
Reviewed Changes
Copilot reviewed 3 out of 15 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/cases/compiler/unionRelationshipCheckPasses3.ts | New test case to verify union assignment errors with different optional property flags |
| tests/cases/compiler/unionRelationshipCheckPasses2.ts | Additional test confirming error handling with union discriminants |
| src/compiler/checker.ts | Updated property comparison logic to consider exactOptionalPropertyTypes when checking optional properties |
Files not reviewed (12)
- tests/baselines/reference/unionRelationshipCheckPasses2(exactoptionalpropertytypes=false).errors.txt: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses2(exactoptionalpropertytypes=false).symbols: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses2(exactoptionalpropertytypes=false).types: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses2(exactoptionalpropertytypes=true).errors.txt: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses2(exactoptionalpropertytypes=true).symbols: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses2(exactoptionalpropertytypes=true).types: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses3(exactoptionalpropertytypes=false).errors.txt: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses3(exactoptionalpropertytypes=false).symbols: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses3(exactoptionalpropertytypes=false).types: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses3(exactoptionalpropertytypes=true).errors.txt: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses3(exactoptionalpropertytypes=true).symbols: Language not supported
- tests/baselines/reference/unionRelationshipCheckPasses3(exactoptionalpropertytypes=true).types: Language not supported
| if (sourceProperty === targetProperty) continue; | ||
| // We compare the source property to the target in the context of a single discriminant type. | ||
| const related = propertyRelatedTo(source, target, sourceProperty, targetProperty, _ => combination[i], /*reportErrors*/ false, IntersectionState.None, /*skipOptional*/ strictNullChecks || relation === comparableRelation); | ||
| const related = propertyRelatedTo(source, target, sourceProperty, targetProperty, _ => combination[i], /*reportErrors*/ false, IntersectionState.None, /*skipOptional*/ strictNullChecks && !exactOptionalPropertyTypes || relation === comparableRelation); |
There was a problem hiding this comment.
skipOptional got introduced in #38101 . According to that PR's description its main goal is to align with this principle:
an optional undefined is the same as an undefined field in everything except what we require be written in declarations
But that's not true under exactOptionalPropertyTypes. So the easiest fix to this issue seems to be to just pass skipOptional === false when exactOptionalPropertyTypes is enabled.
|
@typescript-bot test it |
|
Hey @RyanCavanaugh, the results of running the DT tests are ready. Everything looks the same! |
|
@RyanCavanaugh Here are the results of running the user tests with tsc comparing Everything looks good! |
|
@RyanCavanaugh Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@RyanCavanaugh Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
5ecc087 to
7d5097b
Compare
…oDiscriminatedType
27d8240 to
05f69c2
Compare
fixes #61678